Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance optimizations #491

Merged
merged 3 commits into from
Nov 1, 2020
Merged

Performance optimizations #491

merged 3 commits into from
Nov 1, 2020

Conversation

juanjoDiaz
Copy link
Collaborator

This undoes the performance optimizations that were introduced in #360 and #378.

It seems that v8 has improved significantly: support is a lot better and native implementation of the methods are consistently much faster in all browsers, Node, and Deno.

It can be easily tested using:

function flattenReducer(acc, arr) {
  try {
    // This is faster but susceptible to `RangeError: Maximum call stack size exceeded`
    acc.push(...arr);
    return acc;
  } catch (err) {
    // Fallback to a slower but safer option
    return acc.concat(arr);
  }
}

let start = performance.now();
Array(100).fill(Array(1000).fill(1)).reduce(flattenReducer);
let end = performance.now();
console.log(`Time: ${end - start} ms.`);


start = performance.now();
Array(100).fill(Array(1000).fill(1)).flat();
end = performance.now();
console.log(`Time: ${end - start} ms.`);
function fastJoin(arr, separator) {
  let isFirst = true;
  return arr.reduce((acc, elem) => {
    if (elem === null || elem === undefined) {
      elem = '';
    }

    if (isFirst) {
      isFirst = false;
      return `${elem}`;
    }

    return `${acc}${separator}${elem}`;
  }, '');
}

let size = 1000
let start = performance.now();
fastJoin(Array(size).fill('test'), '-');
let end = performance.now();
console.log(`Time: ${end - start} ms.`);


start = performance.now();
Array(size).fill('test').join('-');
end = performance.now();
console.log(`Time: ${end - start} ms.`);

for Node, you need to import performance doing const { performance } = require('perf_hooks');

@coveralls
Copy link

coveralls commented Nov 1, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 7f35776 on juanjoDiaz:performance_optimizations into 4c74c1e on zemirco:master.

@knownasilya
Copy link
Collaborator

I wonder if we could add a perf test to see if things change with updates or just with node updates

@juanjoDiaz
Copy link
Collaborator Author

There is #365 about perf tests.
I've just never found something like coveralls but for performance. To compare different engines and to avoid regressions.
I'm open to ideas.

We should merge this anyway unless there is any blocker. I have some other PRs coming after this.

@knownasilya knownasilya merged commit 471f5a7 into zemirco:master Nov 1, 2020
@juanjoDiaz juanjoDiaz deleted the performance_optimizations branch November 9, 2020 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants